Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not bind the Management port into the generated Service resource #33694

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

Sgitario
Copy link
Contributor

@Sgitario Sgitario commented May 30, 2023

Related to #33307, task 4.

Yet if users set the "target-port" to "management", this port won't be unbound. I wanted to open this possibility to users that for some reason do want to expose the management port, so let me know what you think about this @iocanel @cescoffier

Related to quarkusio#33307, task 4.

Yet if users set the target-port to "management", this port won't be unbound.
@quarkus-bot
Copy link

quarkus-bot bot commented May 30, 2023

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with dot

This message is automatically generated by a bot.

@Sgitario Sgitario changed the title Do not bind the Management port into the generated Service resource. Do not bind the Management port into the generated Service resource May 30, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented May 30, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that idea. We have a sane default, and the user can override it.

@gsmet
Copy link
Member

gsmet commented May 30, 2023

Is it something that should be backported to 3.1?

@cescoffier
Copy link
Member

@gsmet it can wait 3.2

Copy link
Contributor

@iocanel iocanel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Sgitario Sgitario merged commit e6f9db0 into quarkusio:main Jun 1, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 1, 2023
@Sgitario Sgitario deleted the 32225_followup branch June 1, 2023 12:32
Sgitario added a commit to Sgitario/quarkus that referenced this pull request Jun 1, 2023
After quarkusio#33696 being merged, the port https is no longer bound by default.
The problem is that I merged quarkusio#33694 without rebasing and these two tests needed to be updated.
sberyozkin pushed a commit to sberyozkin/quarkus that referenced this pull request Jun 21, 2023
After quarkusio#33696 being merged, the port https is no longer bound by default.
The problem is that I merged quarkusio#33694 without rebasing and these two tests needed to be updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants